Forward-port downstream pcie-brcmstb patches to 6.12 #6714
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Author
|
@P33M this doesn't actually boot. Can you take a look. Failed boot: |
Contributor
|
Most likely the upstream MSI-X target driver is nonfunctional on a downstream devicetree. I shall investigate. |
Contributor
|
It looks like the series adding upstream DT support for 2712 in pcie-brcmstb (and the MIP) is missing from rpi-6.12.y - I'll see if I can piece that together |
Contributor
|
These commits are missing: This is made awkward as both branches are being rebased, so these SHA1s won't be valid after either one gets a push. It's also awkward as cherry-picking leads to conflicts on aae0d86ce128 and 11ac52dbbc1d . |
This reverts commit 0cdb55e.
…eset" This reverts commit 7e5b7de.
This reverts commit cd75a07.
…or downstream device"" This reverts commit c6c97d4.
Instead of copying fields from pcie_cfg_data structure to brcm_pcie reference it directly. Signed-off-by: Stanimir Varbanov <[email protected]> Reviewed-by: Florian Fainelil <[email protected]>
The BCM2712 memory map can support up to 64GB of system memory, thus expand the inbound window size in calculation helper function. The change is safe for the currently supported SoCs that have smaller inbound window sizes. Signed-off-by: Stanimir Varbanov <[email protected]> Reviewed-by: Florian Fainelli <[email protected]> Reviewed-by: Jim Quinlan <[email protected]> Tested-by: Ivan T. Ivanov <[email protected]> Link: https://lore.kernel.org/r/[email protected] [kwilczynski: commit log] Signed-off-by: Krzysztof Wilczyński <[email protected]>
Add bare minimum amount of changes in order to support PCIe RC hardware IP found on RPi5. The PCIe controller on bcm2712 is based on bcm7712 and as such it inherits register offsets, perst, bridge_reset ops and inbound windows count. Although, the implementation for bcm2712 needs a workaround related to the control of the bridge_reset where turning off of the root port must not shutdown the bridge_reset and this must be avoided. To implement this workaround a quirks field is introduced in pcie_cfg_data struct. Signed-off-by: Stanimir Varbanov <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
The default input reference clock for the PHY PLL is 100Mhz, except for some devices where it is 54Mhz like bcm2712C1 and bcm2712D0. To implement this adjustments introduce a new .post_setup op in pcie_cfg_data and call it at the end of brcm_pcie_setup function. The bcm2712 .post_setup callback implements the required MDIO writes that switch the PLL refclk and also change PHY PM clock period. Without this RPi5 PCIex1 is unable to enumerate endpoint devices on the expansion connector. Signed-off-by: Stanimir Varbanov <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
In case brcmstb PCIe driver and MIP MSI-X interrupt controller drivers are built as modules there could be a race in probing. To avoid this add a softdep to MIP driver to guarantee that MIP driver will be load first. Signed-off-by: Stanimir Varbanov <[email protected]> Reviewed-by: Florian Fainelli <[email protected]> Tested-by: Ivan T. Ivanov <[email protected]> Link: https://lore.kernel.org/r/[email protected] [kwilczynski: commit log] Signed-off-by: Krzysztof Wilczyński <[email protected]>
A call to of_parse_phandle() increments refcount, of_node_put must be called when done the work on it. Fix missing of_node_put() on the msi_np device node by using scope based of_node_put() cleanups. Cc: [email protected] # v5.10+ Fixes: 40ca1bf ("PCI: brcmstb: Add MSI support") Signed-off-by: Stanimir Varbanov <[email protected]>
When the user elects to limit the PCIe generation via the appropriate DT property, apply the settings before the PCIe link-up, not after. Fixes: c045213 ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver") Signed-off-by: Jim Quinlan <[email protected]> Fixes: c045213 ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver") Reviewed-by: Manivannan Sadhasivam <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
The driver was mistakenly writing to a RO config-space register (PCI_EXP_LNKCAP). Although harmless in this case, the proper destination is an internal RW register that is reflected by PCI_EXP_LNKCAP. Fixes: c045213 ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver") Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Manivannan Sadhasivam <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
When setting a register field it was assumed that the field started at the lsb of the register. Although the masks do indeed start at the lsb, and this will probably not change, it is prudent to use a method that makes no assumption about the mask's placement in the register. The uXXp_replace_bits() calls are used since they are already prevalent in this driver. Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Manivannan Sadhasivam <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
If regulator_bulk_get() returns an error, no regulators are created and we need to set their number to zero. If we do not do this and the PCIe link-up fails, regulator_bulk_free() will be invoked and effect a panic. Also print out the error value, as we cannot return an error upwards as Linux will WARN on an error from add_bus(). Fixes: 9e6be01 ("PCI: brcmstb: Enable child bus device regulators from DT") Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
Our system for enabling and disabling regulators is designed to work only on the port driver below the root complex. The conditions to discriminate for this case should be the same when we are adding or removing the bus. Without this change the regulators may be disabled prematurely when a bus further down the tree is removed. Fixes: 9e6be01 ("PCI: brcmstb: Enable child bus device regulators from DT") Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Manivannan Sadhasivam <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
The constants EXT_CFG_DATA and EXT_CFG_INDEX vary by SOC. One of the map_bus methods used these constants, the other used different constants. Fortunately there was no problem because the SoCs that used the latter map_bus method all had the same register constants. Remove the redundant constants and adjust the code to use them. In addition, update EXT_CFG_DATA to use the 4k-page based config space access system, which is what the second map_bus method was already using. Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
The HW team has decided to "tighten" some field definitions in the MDIO packet format. Fortunately these two changes may be made in a backwards compatible manner. The CMD field used to be 12 bits and now is one. This change is backwards compatible because the field's starting bit position is unchanged and the only commands we've used have values 0 and 1. The PORT field's width has been changed from four to five bits. When written, the new bit is not contiguous with the other four. Fortunately, this change is backwards compatible because we have never used anything other than 0 for the port field's value. Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Manivannan Sadhasivam <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
Just make it clear to the reader that there is a conversion happening, in this case from an int type to an irq_hw_number_t, an unsigned long int. Signed-off-by: Jim Quinlan <[email protected]> Reviewed-by: Manivannan Sadhasivam <[email protected]> Reviewed-by: Florian Fainelli <[email protected]>
These chips use a UBUS-AXI bridge component that has configurable timeout and error response handling. Suppress AXI error responses to CPU requests, otherwise these are fatal if they reach the ARM cluster, and set reasonably large timeouts for both Mem and Cfg requests. Signed-off-by: Jonathan Bell <[email protected]>
In commit b478e16 ("PCI/ASPM: Consolidate link state defines") PCIE_LINK_STATE_L1 and PCIE_LINK_STATE_L0s grew some bits for more granular control of ASPM. This broke the aspm-no-l0s override, instead disabling link ASPM completely if this DT property was specified. Specify the field bits in the driver. Fixes: caab002 ("PCI: brcmstb: Disable L0s component of ASPM if requested") Fixes: 0693b42 ("PCI: brcmstb: Split post-link up initialization to brcm_pcie_start_link()") Signed-off-by: Jonathan Bell <[email protected]>
It appears that bits in the Root Control Register are reset with perst_n, which means the PCI layer's call to enable CRS prior to adding/scanning the bus has no effect. Open-code the enable in brcm_pcie_start_link as a workaround. Without CRS visibility, configuration reads issued by the CPU don't retire if the endpoint returns a CRS response - the RC will poll until a (large) timeout is reached. This means the core can stall for a long time during boot. Signed-off-by: Jonathan Bell <[email protected]>
The PHY MDIO register map is different on BCM2712, and as the PHY input clock is 54MHz not 100MHz, enabling refclk SSC is both broken and unfixable. Mask out attempts to enable SSC with a controller quirk. Signed-off-by: Jonathan Bell <[email protected]>
The BCM2712 root complexes can interpret priority signalling in two different ways, based on the incoming Traffic Class of a TLP. The TLP TCs are assigned to separate internal request/response queues, and assigned different AXI IDs. These queues can have outgoing AXI transactions tagged based on: - Static QoS values - Dynamic QoS through internal backpressure - Dynamic QoS with elevation based on Vendor Messages received by the RC The VDM mechanism is of limited use due to implementation bugs, but the implicit reordering due to separate ID assignment allows higher-priority traffic from an EP to overtake other traffic in the RC and rest of the system. RP1 assigns TCs based on its internal bus managers, and internally tags read requests to allow out-of-order completions, so these two features operate in concert to provide priority service to e.g. MIPI camera or display traffic. Signed-off-by: Jonathan Bell <[email protected]>
Some endpoints need longer than the minimum Tperst_clk time of 100us that the PCIe specification allows for, as they may need to sequence internal resets off the stable output of internal PLLs prior to removal of fundamental reset. PCIe switches are an especially bad case, in some cases requiring up to 100 milliseconds for stable downstream link behaviour. Parse the DT property brcm,tperst-clk-ms and use this to hold PERST# low during brcm_pcie_start_link(). The BRCM RC typically outputs 200us of stable refclk before deasserting PERST#. By masking/forcing the output signal while deasserting the internal reset, the effect is to extend the length of time that the refclk is active and stable before PERST# is released. The TX lanes will enter the Polling state before PERST# is released, but this appears to be harmless. Signed-off-by: Jonathan Bell <[email protected]>
Update PCIe controller bindings with BCM2712 support. Signed-off-by: Stanimir Varbanov <[email protected]> Acked-by: Rob Herring (Arm) <[email protected]> Reviewed-by: Florian Fainelli <[email protected]> Tested-by: Ivan T. Ivanov <[email protected]> Link: https://lore.kernel.org/r/[email protected] [kwilczynski: commit log] Signed-off-by: Krzysztof Wilczyński <[email protected]>
pelwell
reviewed
Mar 17, 2025
| status = "okay"; | ||
| }; | ||
|
|
||
| &pcie1 { |
Contributor
Contributor
There was a problem hiding this comment.
Remove the duplication (obvs), and add this to bcm2712-ds.dtsi (my collection of downstream patches to bcm2712.dtsi):
&pcie1 {
status = "disabled";
};
| clock-names = "hdmi", "bvb", "audio", "cec"; | ||
| }; | ||
|
|
||
| &pcie1 { |
Contributor
There was a problem hiding this comment.
I don't think we want pcie1 enabled by default.
| status = "okay"; | ||
| }; | ||
|
|
||
| &pcie2 { |
Contributor
There was a problem hiding this comment.
And this is already enabled at rp1_target above.
pelwell
reviewed
Mar 17, 2025
| status = "okay"; | ||
| }; | ||
|
|
||
| &pcie1 { |
Contributor
There was a problem hiding this comment.
The commit message for this looks to be largely vestigial.
Is there any reason this is CM5-specific? Doesn't it belong on bcm2712-ds.dtsi?
Contributor
There was a problem hiding this comment.
It's not specific to either model (and inadvisable to change - needs to match up with firmware setup of the other bus priorities). So it can go in the base downstream dtb now.
Collaborator
Author
|
I've pushed some commits related to comments - not quite sure if I've interpreted them correctly. |
Collaborator
Author
|
@pelwell any more changes needed? I can squash the fixups before merging. |
Contributor
|
I'm going to struggle to read through it thoroughly again, but I can't see anything obviously wrong now. |
pelwell
approved these changes
Mar 18, 2025
popcornmix
force-pushed
the
pcie_612
branch
2 times, most recently
from
4faafa1 to
b580f43
Compare
Collaborator
Author
|
Squashed. There was a request to move |
Contributor
|
I'd say yes - both are downstream properties. |
Contributor
|
Yes. And this is still wrong in bcm2712-rpi-5-b.dts: pcie1 should not be enabled by default. Is the |
Contributor
|
It should be specific to Pi 5 x1 as we don't know that there aren't any expansion boards out there that don't wire up CLKREQ, especially ones that went to market before publication of the HAT+ spec. So the old behaviour needs to be preserved (where absence of brcm,enable-l1ss implies CLKREQ is ignored, and presence treats it as a bidirectional pin). CM4 has always had L1SS enabled, and CM5 should follow suit. |
pcie1 should use the FIFO threshold property as the RC should not pay attention to Vendor Messages from incompatible endpoint hardware. Also drop the downstream MPS property, it's no longer needed. Signed-off-by: Jonathan Bell <[email protected]>
Enable pcie1 and pcie2 DT nodes. Pcie1 is used for the extension connector and pcie2 is used for RP1 south-bridge. Signed-off-by: Stanimir Varbanov <[email protected]>
Remove some gratuitous differences with the upstream dts, and drop the unnecessary 'status = "okay"' properties (they are needed(*) to override 'status = "disabled"' in the original definitions. (*) You could technically delete the original properties, but that looks worse and doesn't work in an overlay. Signed-off-by: Phil Elwell <[email protected]>
About to add the upstream PCIe nodes, so remove the downstream ones to avoid duplicated nodes and build breakage. Signed-off-by: Dave Stevenson <[email protected]>
Add PCIe devicetree nodes, plus needed reset and mip MSI-X controllers. Signed-off-by: Stanimir Varbanov <[email protected]>
Add an interrupt controller driver for MSI-X Interrupt Peripheral (MIP) hardware block found in bcm2712. The interrupt controller is used to handle MSI-X interrupts from peripherials behind PCIe endpoints like RP1 south bridge found in RPi5. There are two MIPs on bcm2712, the first has 64 consecutive SPIs assigned to 64 output vectors, and the second has 17 SPIs, but only 8 of them are consecutive starting at the 8th output vector. Signed-off-by: Stanimir Varbanov <[email protected]>
Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller. Signed-off-by: Stanimir Varbanov <[email protected]>
L1 sub-state support and clkreq control are intermingled in the hardware, and upstream now have a tri-state property to control this. The default behaviour is now to enable clkreq control if the property is absent. Default Pi 5 PCIex1 to "safe" to avoid breaking expansion boards without the CLKREQ# line connected. Make this explicit in the pciex1-compat-pi5 overlay, and remove the obsolete CM4 property. Signed-off-by: Jonathan Bell <[email protected]>
P33M
approved these changes
Mar 18, 2025
Collaborator
Author
|
Updated. |
pelwell
approved these changes
Mar 18, 2025
popcornmix
added a commit
to raspberrypi/firmware
that referenced
this pull request
Mar 18, 2025
See: raspberrypi/linux#6714 kernel: Fixup HyperPixel4 display on 6.12 See: raspberrypi/linux#6722 kernel: overlays: Fix some unusable fragments
popcornmix
added a commit
to raspberrypi/rpi-firmware
that referenced
this pull request
Mar 18, 2025
See: raspberrypi/linux#6714 kernel: Fixup HyperPixel4 display on 6.12 See: raspberrypi/linux#6722 kernel: overlays: Fix some unusable fragments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
6.12 and 6.14 trees have quite different code for pcie.
It will help support going forward to use the same base, so backport #6675 to 6.12.